Skip to content

Make DynamicPPL.TestUtils.run_ad return both the primal and gradient benchmarks #1009

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Aug 11, 2025
Merged

Make DynamicPPL.TestUtils.run_ad return both the primal and gradient benchmarks #1009

merged 1 commit into from
Aug 11, 2025
+40 −14

Conversation

penelopeysm
Copy link
Member

@penelopeysm penelopeysm commented Aug 4, 2025 •
edited
Loading

run_ad(...; benchmark=true) used to benchmark logdensity and logdensity_and_gradient, then take the ratio and return that.

This PR makes it return both benchmark results because, I mean, why not? We already have this information.

There is a bit of dangerous scope creep here in that this function is turning into a replacement for TuringBenchmarking. (Note it can't benchmark the primal alone without a gradient though so it's still not the same.)

But I think it's still useful to know both figures separately. For example, in the CI benchmarks table, we tabulate both times.

Unfortunately I attempted to change the CI benchmark setup to use the new functionality #1002 and promptly got bitten by the bug in #1001. So this PR doesn't change that, not for now, at least.

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Aug 4, 2025 •
edited
Loading

Benchmark Report for Commit 2c4d913

Computer Information

Julia Version 1.11.6
Commit 9615af0f269 (2025-07-09 12:58 UTC)
Build Info:
  Official https://julialang.org/ release
Platform Info:
  OS: Linux (x86_64-linux-gnu)
  CPU: 4 × AMD EPYC 7763 64-Core Processor
  WORD_SIZE: 64
  LLVM: libLLVM-16.0.6 (ORCJIT, znver3)
Threads: 1 default, 0 interactive, 1 GC (on 4 virtual cores)

Benchmark Results

|                 Model | Dimension |  AD Backend |      VarInfo Type | Linked | Eval Time / Ref Time | AD Time / Eval Time |
|-----------------------|-----------|-------------|-------------------|--------|----------------------|---------------------|
| Simple assume observe |         1 | forwarddiff |             typed |  false |                  8.7 |                 1.5 |
|           Smorgasbord |       201 | forwarddiff |             typed |  false |                637.7 |                45.3 |
|           Smorgasbord |       201 | forwarddiff | simple_namedtuple |   true |                409.4 |                55.7 |
|           Smorgasbord |       201 | forwarddiff |           untyped |   true |               1024.6 |                33.9 |
|           Smorgasbord |       201 | forwarddiff |       simple_dict |   true |               6291.5 |                26.7 |
|           Smorgasbord |       201 | reversediff |             typed |   true |               1429.6 |                28.6 |
|           Smorgasbord |       201 |    mooncake |             typed |   true |               1021.0 |                 4.2 |
|    Loop univariate 1k |      1000 |    mooncake |             typed |   true |               5641.1 |                 4.0 |
|       Multivariate 1k |      1000 |    mooncake |             typed |   true |                962.7 |                 9.0 |
|   Loop univariate 10k |     10000 |    mooncake |             typed |   true |              63367.8 |                 3.7 |
|      Multivariate 10k |     10000 |    mooncake |             typed |   true |               8449.5 |                 9.8 |
|               Dynamic |        10 |    mooncake |             typed |   true |                130.8 |                12.0 |
|              Submodel |         1 |    mooncake |             typed |   true |                 13.1 |                 4.6 |
|                   LDA |        12 | reversediff |             typed |   true |               1008.7 |                 3.1 |

@penelopeysm penelopeysm requested a review from sunxd3 August 4, 2025 21:30
@codecov Codecov
Copy link

codecov bot commented Aug 4, 2025 •
edited
Loading

Codecov Report

❌ Patch coverage is 28.57143% with 5 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (main@084f5af). Learn more about missing BASE report.
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
src/test_utils/ad.jl 28.57% 5 Missing ⚠️
Additional details and impacted files 
@@           Coverage Diff           @@
##             main    #1009   +/-   ##
=======================================
  Coverage        ?   82.26%           
=======================================
  Files           ?       38           
  Lines           ?     3947           
  Branches        ?        0           
=======================================
  Hits            ?     3247           
  Misses          ?      700           
  Partials        ?        0

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Base automatically changed from breaking to main August 7, 2025 09:45
@sunxd3
Copy link
Member

sunxd3 commented Aug 11, 2025

@penelopeysm sorry for missing this!

is it a good time to review it?

@penelopeysm
Copy link
Member Author

@sunxd3 Sure, now is as good a time as any other :)

@sunxd3
Copy link
Member

sunxd3 commented Aug 11, 2025

there are quite a lot of code changes now, should we do a rebase of this branch to something? sorry for asking

@penelopeysm
Copy link
Member Author

Oh, dear, sorry. I'll sort that.

@github-actions GitHub Actions
Copy link
Contributor

DynamicPPL.jl documentation for PR #1009 is available at:
https://TuringLang.github.io/DynamicPPL.jl/previews/PR1009/

sunxd3
sunxd3 approved these changes Aug 11, 2025
View reviewed changes
Copy link
Member

@sunxd3 sunxd3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks great! ready to go from my side once CI passes

@coveralls
Copy link

coveralls commented Aug 11, 2025 •
edited
Loading

Pull Request Test Coverage Report for Build 16876123332

Details

  • 0 of 7 (0.0%) changed or added relevant lines in 1 file are covered.
  • 31 unchanged lines in 8 files lost coverage.
  • Overall coverage decreased (-0.04%) to 82.516%
Changes Missing Coverage Covered Lines Changed/Added Lines %
src/test_utils/ad.jl 0 7 0.0%
Files with Coverage Reduction New Missed Lines %
src/bijector.jl 1 93.33%
src/model.jl 1 86.73%
src/debug_utils.jl 3 89.27%
src/extract_priors.jl 3 60.53%
src/pointwise_logdensities.jl 3 93.06%
src/varinfo.jl 3 85.33%
src/values_as_in_model.jl 4 72.41%
src/threadsafe.jl 13 62.16%
Totals Coverage Status
Change from base Build 16862061135: -0.04%
Covered Lines: 3247
Relevant Lines: 3935
💛 - Coveralls

@penelopeysm
Copy link
Member Author

Thanks @sunxd3!

@penelopeysm penelopeysm merged commit 8fb70d4 into main Aug 11, 2025
20 of 21 checks passed
@penelopeysm penelopeysm deleted the py/better-run-ad branch August 11, 2025 10:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sunxd3 sunxd3 sunxd3 approved these changes

Assignees

@penelopeysm penelopeysm

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@penelopeysm @sunxd3 @coveralls